Skip to content

Conversation

@dnguyen227
Copy link
Contributor

@dnguyen227 dnguyen227 commented Jan 8, 2026

This PR fixes the MBM implementation to use the tightest M values possible. This means that each disjunct constraint has it's own M value associated with any other disjunct. Originally only disjuncts has corresponding M values.

EXAMPLE

Given a disjunction with:

  • Y[1]: x <= 2 and x >= 1
  • Y[2]: 0 <= x <= 55

When reformulating Y[1]'s constraints with respect to Y[2]'s feasible region (0 ≤ x ≤ 55):

  • x <= 2: max(x - 2) → M = 53 (at x = 55)
  • x >= 1: max(1 - x) → M = 1 (at x = 0)

Before (max M of possible constraints):
x - 53·Y[2] <= 2
x + 53·Y[2] >= 1 ← Both use M = max(53, 1) = 53

After (per-constraint):
x - 53·Y[2] <= 2 ← Uses its own M = 53
x + 1·Y[2] >= 1 ← Uses its own M = 1 (tighter)

The code is largely the same. The change made is maximum(_maximize_M(...)) is now just _maximize_M(...), and order of for loops was adjusted to reflect this.

Relevant tests were added to check for these unique M values in addition to verbose comments.

@codecov
Copy link

codecov bot commented Jan 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.35%. Comparing base (3f167c0) to head (d1c9fb6).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #132      +/-   ##
==========================================
- Coverage   99.35%   99.35%   -0.01%     
==========================================
  Files          17       17              
  Lines        1858     1855       -3     
==========================================
- Hits         1846     1843       -3     
  Misses         12       12              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dnguyen227 dnguyen227 marked this pull request as ready for review January 8, 2026 20:22
@dnguyen227 dnguyen227 marked this pull request as draft January 8, 2026 20:27
@pulsipher pulsipher added the enhancement New feature or request label Jan 8, 2026
@dnguyen227 dnguyen227 marked this pull request as ready for review January 9, 2026 03:46
M::Dict{LogicalVariableRef{M}, T}
default_M::T
conlvref::Vector{LogicalVariableRef{M}}
M::Dict{LogicalVariableRef{M}, Union{T, Vector{T}}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now storing M values for vector constraints as vectors to reflect unique values in each of rows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants